fix(build): stop dts emit leaking @commercetools-frontend/ui-kit into published types#3254
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 7f3ccdf The changes in this PR will be included in the next version bump. This PR includes changesets to release 97 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
valoriecarli
approved these changes
Jun 10, 2026
21e0177 to
72a7a9d
Compare
tdeekens
approved these changes
Jun 11, 2026
valoriecarli
approved these changes
Jun 11, 2026
… published types
The compound components (Constraints, RadioInput, ViewSwitcher) emitted
import("@commercetools-frontend/ui-kit").T...Props in their published .d.ts.
Strict consumers installing only the granular @commercetools-uikit/* packages
don't have the aggregate preset, so the reference resolved to any (TS7006).
Root cause: @commercetools-frontend/ui-kit is a root devDependency (so the
.visualroute/bundlespec fixtures resolve under strict pnpm), which makes the
bare specifier resolvable during preconstruct's declaration emit, and TS prefers
it over the in-package relative path. The build now hides that symlink for the
preconstruct build step only (trap-guarded restore), forcing the correct
relative specifier. No component API or source changed.
Refs: commercetools/identity#757
Add a post-emit regression guard to scripts/build.sh that fails the build if any granular package's published declarations reference the aggregate @commercetools-frontend/ui-kit preset by its bare specifier. Future-proofs the FEC-938 declaration-emit workaround against being dropped, preconstruct changing resolution, or a new compound component reintroducing the pattern. Also reword the changeset to be consumer-facing.
e3479a7 to
7f3ccdf
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The published
.d.tsof three compound components —@commercetools-uikit/constraints,@commercetools-uikit/radio-input, and@commercetools-uikit/view-switcher— leak a reference to the aggregate preset package:Strict-TS consumers that install only the granular
@commercetools-uikit/*packages don't have@commercetools-frontend/ui-kit, so the reference resolves toany, contextual handler typing is lost, andnoImplicitAnyflags it asTS7006— on what looks like a routine minor bump. This was discovered downstream in commercetools/identity#757 (RadioInput.GrouponChangehandlers).Root cause
Not an API change — source is byte-identical and
tscalone emits the correct relative specifier. WithpreserveSymlinks: false(set in FEC-935 so typecheck resolves@storybook/*subpaths), TypeScript's declaration emit follows pnpm's symlinks, discovers that the leaf type is also reachable through the node_modules package@commercetools-frontend/ui-kit(which re-exports it), and prefers the bare node_modules specifier over the relative path — a deliberate, long-standing TypeScript behavior.That package is only present in
node_modulesbecause it became a rootdevDependency(added in FEC-938 so the.visualroute/bundlespecfixtures resolve under strict pnpm). Verified by toggling the single variable — with the symlink present the emit leaks; with it removed (everything else held constant, includingpreserveSymlinks: false) the emit is correct:main)import("@commercetools-frontend/ui-kit").TGroupProps❌import("./radio-group.js").TGroupProps✅Fix (minimally invasive)
Belt and suspenders:
scripts/build.shhides the meta-package symlink for thepreconstruct buildstep only (preconstruct doesn't need it — the fixtures aren't part of the dts program), then restores it (trap-guarded so it's restored even if the build fails). No source, component API, orpreserveSymlinkschange; the.visualroute/bundlespecfixtures are untouched.find(not a**glob) so it covers packages at every nesting depth.This is the smallest change that produces correct published types, and it's the right trade-off for a legacy codebase: it touches one build script and leaves every TypeScript config, dependency, and source file exactly as it is.
Alternatives considered (and why not)
pathscannot fix this — TypeScript'spathschanges module resolution, never declaration emit. The two structural options were both heavier and were rejected:preserveSymlinks: true(globally, or via a dedicated emit tsconfig). It does fix the specifier, but it reintroduces the full FEC-935 typecheck breakage (~270 errors, including in published component source, not just stories), so it breakspnpm typecheckand the IDE. Decoupling it for emit-only isn't clean either: preconstruct requires a custom-named tsconfig to exist in every package directory (it doesn't walk up to the root), and VS Code can't be pointed at a non-default tsconfig — so the editor would still surface the ~270 errors.@commercetools-frontend/ui-kitdevDependency and repoint the 71.visualroute/bundlespecfixtures off the aggregate onto granular packages. This is the most structurally "correct" fix (the symlink would never exist), but it's 71 mechanical edits plus a Percy visual-regression re-run — disproportionate to a one-line build-step problem.The chosen fix implements the same underlying strategy as the structural option — keep the aggregate package out of the leaf packages' declaration-emit graph — but does it at build time with no churn.
Known limitation
A bare
pnpm preconstruct buildrun outsidescripts/build.shwould still leak, and the regression guard only runs as part ofscripts/build.sh. The release path goes throughpnpm build, so published artifacts are both fixed and guarded.Verification
Full
pnpm buildrun on this branch:radio-input→import("./radio-group.js").TGroupProps,import("./radio-option.js").TOptionPropsconstraints→import("./horizontal/index.js").THorizontalPropsview-switcher→import("./view-switcher.js").TViewSwitcherPropsRelease
Patch changeset included. The cohort is a
fixedchangeset group, so the whole20.6.xline republishes together — every package is rebuilt with the fixed build, so the fix propagates cohort-wide.Refs: commercetools/identity#757